Switch TypeApplications to given extensions #23512

som-snytt · 2025-07-11T23:51:07Z

Casual refactor to try newer features.

som-snytt · 2025-07-12T01:58:36Z

I prefer to assume it's yet another spurious test failure.

[info] Test run dotty.tools.pc.tests.hover.HoverTermSuite started
[info] Test dotty.tools.pc.tests.hover.HoverTermSuite.for-keyword started
Error:  Test dotty.tools.pc.tests.hover.HoverTermSuite.for-keyword failed: java.util.concurrent.CancellationException: null, took 10.636 sec
Error:      at java.util.concurrent.CompletableFuture.cancel(CompletableFuture.java:2276)
Error:      at scala.meta.internal.pc.CompilerAccess.onCompilerJobQueue$$anonfun$3$$anonfun$1(CompilerAccess.scala:231)
Error:      at java.util.concurrent.FutureTask.run(FutureTask.java:266)
Error:      at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:180)
Error:      at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293)
Error:      at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
Error:      at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
Error:      at java.lang.Thread.run(Thread.java:748)

There is an infrastructure problem with so many recent failures.

pc tests are inherently flaky if they are reactive. It would be nice to have tiers of unit tests, functional tests (integration), and liveliness tests that might time out but which would be run in a quiescent environment maybe once a day.

odersky · 2025-07-13T12:26:51Z

compiler/src/dotty/tools/dotc/core/Types.scala

@@ -2132,6 +2132,9 @@ object Types extends TypeUtils {
    /** Is the `hash` of this type the same for all possible sequences of enclosing binders? */
    def hashIsStable: Boolean = true
  }
+  object Type:


No sure about performance impact here. Do we need the given? Why can't we put the extension methods directly in object TypeApplications?

The problem with compiler refactoring is that we have to be super careful not to make things slower. TypeApplications methods are very often called, so it should be ideally obvious that the new scheme is not slower than the old one.

Yes, the experiment is to understand the difference in current and export in Types (cost is a forwarder) and this pr (extra reference).

The goal was to avoid import TypeApplications.* everywhere (the first edit) because I'm too lazy to add that everywhere.

So this is also a test of which idiom is optimal. (Maybe importing everywhere is OK, modulo PR delta.)

As a coder, should I have to guess or even benchmark?

The problem is it's notoriously noisy to benchmark the compiler. Every single commit is in the noise but in the aggregate compilation times go up. And it's almost impossible to bring them down again. Many people have tried and failed to move the needle. I had some limited successes that I obtained by spending months of concentrated work.

So when working with the compiler, the guideline has to be: niceties are irrelevant, always pick the idiom that you believe is fastest. The only exception to this rule: you know that you are working on a part of the compiler that is executed only in rare circumstances (e.g. error message reporting). Then you can splurge on niceties. But TypeApplications is potentially hot.

As a point of illustration: See how often we write a :: Nil instead of List(a). It's not because we think it looks better (it doesn't) but it leads to better code.

som-snytt · 2025-07-14T11:14:19Z

I'm reminded that this caught my eye because of the bootstrap error on JDK 23.

java.lang.BootstrapMethodError: bootstrap method initialization exception
  | => sat dotty.tools.dotc.core.TypeApplications$.typeParams(TypeApplications.scala:206)

som-snytt · 2025-07-14T11:55:30Z

The difference is that value class eliminates the allocation but not the call to the decorator (identity).

In PlainPrinter.homogenize:

       635: getstatic     #795                // Field dotty/tools/dotc/core/TypeApplications$.MODULE$:Ldotty/tools/dotc/core/TypeApplications$;
       638: aload         35
       640: aload_0
       641: invokevirtual #491                // Method given_Context:()Ldotty/tools/dotc/core/Contexts$Context;
       644: invokevirtual #798                // Method dotty/tools/dotc/core/Types$Type.dealias:(Ldotty/tools/dotc/core/Contexts$Context;)Ldotty/tools/dotc/core/Types$Type;
       647: aload         36
       649: aload_0
       650: invokevirtual #491                // Method given_Context:()Ldotty/tools/dotc/core/Contexts$Context;
       653: invokevirtual #802                // Method dotty/tools/dotc/core/TypeApplications$.appliedTo:(Ldotty/tools/dotc/core/Types$Type;Lscala/collection/immutable/List;Ldotty/tools/dotc/core/Contexts$Context;)Ldotty/tools/dotc/core/Types$Type;
       656: areturn

was

       605: getstatic     #795                // Field dotty/tools/dotc/core/TypeApplications$.MODULE$:Ldotty/tools/dotc/core/TypeApplications$;
       608: getstatic     #800                // Field dotty/tools/dotc/core/Types$.MODULE$:Ldotty/tools/dotc/core/Types$;
       611: aload         28
       613: aload_0
       614: invokevirtual #491                // Method given_Context:()Ldotty/tools/dotc/core/Contexts$Context;
       617: invokevirtual #803                // Method dotty/tools/dotc/core/Types$Type.dealias:(Ldotty/tools/dotc/core/Contexts$Context;)Ldotty/tools/dotc/core/Types$Type;
       620: invokevirtual #806                // Method dotty/tools/dotc/core/Types$.decorateTypeApplications:(Ldotty/tools/dotc/core/Types$Type;)Ldotty/tools/dotc/core/Types$Type;
       623: aload         29
       625: aload_0
       626: invokevirtual #491                // Method given_Context:()Ldotty/tools/dotc/core/Contexts$Context;
       629: invokevirtual #810                // Method dotty/tools/dotc/core/TypeApplications$.appliedTo$extension:(Ldotty/tools/dotc/core/Types$Type;Lscala/collection/immutable/List;Ldotty/tools/dotc/core/Contexts$Context;)Ldotty/tools/dotc/core/Types$Type;
       632: areturn

The Context is not an arg but a member def here.

som-snytt · 2025-07-14T18:49:27Z

Testing stack traces will always be brittle. In this case, the stack didn't even change, it was the line number.

scala.quoted.runtime.impl.QuotesImpl$reflect$StringConstant$.apply(QuotesImpl.scala:2535)

This problem is addressed by
#22389
which was not yet addressed. It is a baby step toward parity with Par-Test.

som-snytt · 2025-07-15T17:58:59Z

If -gc means what I think it means, testing collection will always be brittle. One can test reachability.

Error:    - test 'tests\run\lambda-serialization-gc.scala' timed out, took 214.346 sec

Is that a lot of seconds?

som-snytt · 2025-07-15T18:26:02Z

The test wants to force OOM by allocating a good chunk of what's available, but max memory is arbitrarily unconstrained.

        val maxMemory = Runtime.getRuntime.maxMemory()
        val junk = new Array[Long]((maxMemory / 8 / 4).toInt)

The class loader allocates, to see if lambda serialization causes it not to be reclaimed.

It could cause unnecessary swapping etc as it asks for more memory from the OS. (Speculating.) So while the test is try to "fail quickly", it could be incurring slow mechanisms that cause it not to complete expeditiously.

I don't even recall off the cuff if the test rig is forking here. Partest would fork in the presence of .jvmopts or whatever. I see one tests/run/bridges.javaopts.

The test does not System.gc(), so it's relying on memory pressure to cause gc? (Speculating.) So even in the absence of a leak, it could be allocating on subsequent "rounds" of the serialization test.

The test is very close to the Scala 2 test. It can't be run by scala-cli in modern jvm because of the assumption about the class loader.

Exception in thread "main" java.lang.ClassCastException: class jdk.internal.loader.ClassLoaders$AppClassLoader cannot be cast to class java.net.URLClassLoader (jdk.internal.loader.ClassLoaders$AppClassLoader and java.net.URLClassLoader are in module java.base of loader 'bootstrap')

som-snytt · 2025-07-15T18:35:19Z

The output says

Picked up _JAVA_OPTIONS: -Xmx8192m -XX:ReservedCodeCacheSize=512m -XX:MaxMetaspaceSize=1024m

I'll look at the test separately. (It looks like it's testing a caching feature not implemented in Scala 3.)

Worth adding that the github runner experience is pretty nice; in particular, just re-running the failed windows test job.

som-snytt · 2025-07-15T19:54:48Z

Maybe I broke something.

On scala 2, I'd previously added to the test

//> using javaOpt -Xmx512m

som-snytt · 2025-07-15T19:56:15Z

C.I. is just broken.

som-snytt marked this pull request as ready for review July 12, 2025 01:58

odersky reviewed Jul 13, 2025

View reviewed changes

som-snytt marked this pull request as draft July 14, 2025 10:09

som-snytt added 2 commits July 14, 2025 04:20

Switch TypeApplications to exported extensions

aafa25c

Switch TypeApplications from export to given

253ccb9

som-snytt force-pushed the topic/type-applications-extension branch from fa9c3cc to 4da3dd1 Compare July 14, 2025 11:20

som-snytt marked this pull request as ready for review July 14, 2025 12:09

Switch TypeApplications to extensions for import

b0ff516

som-snytt force-pushed the topic/type-applications-extension branch from 4da3dd1 to b0ff516 Compare July 14, 2025 12:24

som-snytt mentioned this pull request Jul 14, 2025

Insert spaces when doing rewrite _ to ? #23449

Closed

Update checks

2d48989

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Switch TypeApplications to given extensions #23512

Switch TypeApplications to given extensions #23512

som-snytt commented Jul 11, 2025

Uh oh!

som-snytt commented Jul 12, 2025

Uh oh!

odersky Jul 13, 2025

Uh oh!

som-snytt Jul 13, 2025

Uh oh!

odersky Jul 13, 2025

Uh oh!

som-snytt commented Jul 14, 2025

Uh oh!

som-snytt commented Jul 14, 2025

Uh oh!

som-snytt commented Jul 14, 2025 •

edited

Loading

Uh oh!

som-snytt commented Jul 15, 2025

Uh oh!

som-snytt commented Jul 15, 2025

Uh oh!

som-snytt commented Jul 15, 2025 •

edited

Loading

Uh oh!

som-snytt commented Jul 15, 2025

Uh oh!

som-snytt commented Jul 15, 2025

Uh oh!

Uh oh!

Switch TypeApplications to given extensions #23512

Are you sure you want to change the base?

Switch TypeApplications to given extensions #23512

Conversation

som-snytt commented Jul 11, 2025

Uh oh!

som-snytt commented Jul 12, 2025

Uh oh!

odersky Jul 13, 2025

Choose a reason for hiding this comment

Uh oh!

som-snytt Jul 13, 2025

Choose a reason for hiding this comment

Uh oh!

odersky Jul 13, 2025

Choose a reason for hiding this comment

Uh oh!

som-snytt commented Jul 14, 2025

Uh oh!

som-snytt commented Jul 14, 2025

Uh oh!

som-snytt commented Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

som-snytt commented Jul 15, 2025

Uh oh!

som-snytt commented Jul 15, 2025

Uh oh!

som-snytt commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

som-snytt commented Jul 15, 2025

Uh oh!

som-snytt commented Jul 15, 2025

Uh oh!

Uh oh!

som-snytt commented Jul 14, 2025 •

edited

Loading

som-snytt commented Jul 15, 2025 •

edited

Loading